feat(pagination): add cursor-based pagination + client APIs#112
feat(pagination): add cursor-based pagination + client APIs#112
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #112 +/- ##
==========================================
+ Coverage 88.74% 89.34% +0.59%
==========================================
Files 7 8 +1
Lines 622 713 +91
==========================================
+ Hits 552 637 +85
- Misses 70 76 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds cursor-based pagination functionality to pycouchdb, implementing both view and Mango query pagination along with client API methods. The main purpose is to provide stable, memory-efficient pagination without the pitfalls of skip-based pagination.
Key changes:
- Adds a new
pagination.pymodule withview_pages()andmango_pages()functions for cursor-based pagination - Extends the Database class with
find(),view_pages(), andmango_pages()methods - Includes comprehensive test coverage for both unit and integration testing
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pycouchdb/pagination.py | Core pagination logic implementing cursor-based pagination for views and Mango queries |
| pycouchdb/client.py | Database class extensions adding pagination methods and Mango query support |
| pycouchdb/types.py | Type aliases for pagination-related types |
| pycouchdb/utils.py | Enhanced UTF-8 decoding with error handling |
| test/test_pagination.py | Unit tests for pagination functions with mocked dependencies |
| test/test_database.py | Unit tests for new Database methods |
| test/integration/test_pagination_integration.py | Comprehensive integration tests with real CouchDB |
| test/integration/test_integration.py | Additional integration tests for pagination functionality |
| docs/source/quickstart.rst | Documentation updates explaining pagination features |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
pycouchdb/pagination.py
Outdated
| last_row = rows[page_size - 1] | ||
| startkey = last_row['key'] | ||
| startkey_docid = last_row['id'] | ||
| skip = 1 # Skip the row we used as cursor |
There was a problem hiding this comment.
[nitpick] The inline comment explains what skip=1 does, but it would be clearer to explain why this is necessary to avoid duplicate results in cursor-based pagination.
| skip = 1 # Skip the row we used as cursor | |
| skip = 1 # Skip the row used as the cursor to avoid returning it again (prevents duplicate results in cursor-based pagination) |
pycouchdb/pagination.py
Outdated
| def _prepare_mango_data(selector: Dict[str, Any], params: Dict[str, Any]) -> bytes: | ||
| """Prepare Mango query data for HTTP request.""" | ||
| data_dict = { | ||
| 'selector': selector, | ||
| **params | ||
| } | ||
| return utils.force_bytes(json.dumps(data_dict)) |
There was a problem hiding this comment.
The _prepare_mango_data function is defined but never used in the code. Consider removing it or adding a comment explaining its intended future use.
| def _prepare_mango_data(selector: Dict[str, Any], params: Dict[str, Any]) -> bytes: | |
| """Prepare Mango query data for HTTP request.""" | |
| data_dict = { | |
| 'selector': selector, | |
| **params | |
| } | |
| return utils.force_bytes(json.dumps(data_dict)) |
pycouchdb/client.py
Outdated
| :param params: Additional query parameters | ||
| :returns: Iterator yielding lists of rows for each page | ||
|
|
||
| .. versionadded: 1.17 |
There was a problem hiding this comment.
The docstring format should be .. versionadded:: 1.17 with double colons, not single colon.
| .. versionadded: 1.17 | |
| .. versionadded:: 1.17 |
pycouchdb/client.py
Outdated
| :param params: Additional query parameters | ||
| :returns: Iterator yielding lists of documents for each page | ||
|
|
||
| .. versionadded: 1.17 |
There was a problem hiding this comment.
The docstring format should be .. versionadded:: 1.17 with double colons, not single colon.
| .. versionadded: 1.17 | |
| .. versionadded:: 1.17 |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| last_row = rows[page_size - 1] | ||
| startkey = last_row['key'] | ||
| startkey_docid = last_row['id'] | ||
| skip = 1 # Skip the row used as the cursor to avoid returning it again (prevents duplicate results in cursor-based pagination) |
There was a problem hiding this comment.
[nitpick] The comment is overly verbose and could be clearer. Consider simplifying to 'Skip cursor row to avoid duplicates'.
| skip = 1 # Skip the row used as the cursor to avoid returning it again (prevents duplicate results in cursor-based pagination) | |
| skip = 1 # Skip cursor row to avoid duplicates |
| startkey = last_row['key'] | ||
| startkey_docid = last_row['id'] |
There was a problem hiding this comment.
The code uses magic string keys 'key' and 'id' without validation. Consider adding defensive checks or constants for these keys to improve maintainability.
| # Valid JSON but with invalid UTF-8 sequence in the middle | ||
| self.content = b'{"key": "value", "invalid": "\xff\xfe"}' |
There was a problem hiding this comment.
The byte string contains invalid UTF-8 sequences inside a JSON string literal, but this creates invalid JSON, not just invalid UTF-8. The test may not be testing the intended scenario.
| # Valid JSON but with invalid UTF-8 sequence in the middle | |
| self.content = b'{"key": "value", "invalid": "\xff\xfe"}' | |
| # Valid JSON, but the value for "invalid" is a string with bytes that are invalid UTF-8 | |
| self.content = b'{"key": "value", "invalid": "' + b'\xff\xfe'.decode("latin1").encode("utf-8", "surrogateescape") + b'"}' |
| # Valid JSON with invalid UTF-8 that will be replaced with replacement character | ||
| self.content = b'{"key": "value", "invalid": "\xff\xfe\x80"}' |
There was a problem hiding this comment.
Similar to the previous test, this creates invalid JSON syntax rather than testing UTF-8 decode errors. The invalid bytes should be outside the JSON string structure.
No description provided.